feat(agents): collapse denied agent route access to 404 instead of 403#271
Merged
Conversation
685126a to
0d99f31
Compare
e358194 to
eec235f
Compare
eec235f to
0cfb83c
Compare
0d99f31 to
14796e9
Compare
jenniechung
approved these changes
Jun 3, 2026
harvhan
approved these changes
Jun 4, 2026
0cfb83c to
ec8c2ec
Compare
883921e to
6de8f39
Compare
ec8c2ec to
f7e134b
Compare
6de8f39 to
f1f7a42
Compare
2bea751 to
e13cfdb
Compare
f1f7a42 to
7bc61d6
Compare
e13cfdb to
6d88912
Compare
7bc61d6 to
1dd4143
Compare
6d88912 to
bf273c6
Compare
6d14686 to
c00f3e9
Compare
bf273c6 to
68cbdb9
Compare
c00f3e9 to
4c4e73a
Compare
68cbdb9 to
104ecbd
Compare
4c4e73a to
73a5056
Compare
asherfink
added a commit
that referenced
this pull request
Jun 9, 2026
#270) ## Related work Parent ticket: AGX1-242 - AgentEx authorization dual-write. This change is part of a 4-PR stack across 3 repos. | Repo | PR | Purpose | |---|---|---| | scaleapi/scaleapi | [scaleapi/scaleapi#146335](scaleapi/scaleapi#146335) | merged flag registry update for the shared AgentEx resources rollout flags | | scaleapi/agentex | [scaleapi/agentex#364](scaleapi/agentex#364) | agentex-auth routes each auth verb to one backend target | | **scaleapi/scale-agentex** | **this PR** | **registers agentex resources in the authorization graph and grants agent/task ownership** | | scaleapi/scale-agentex | [#271](#271) | denied direct agent access collapses to 404 | **Merge/deploy order:** [scaleapi/scaleapi#146335](scaleapi/scaleapi#146335) is merged; deploy [scaleapi/agentex#364](scaleapi/agentex#364) before this PR, then merge/deploy [#271](#271). Rollout per account is: enable `fgac-agentex-resources-dual-write`, backfill existing resources into Spark, then enable `fgac-agentex-resources` reads. ## What Registers agentex resources in the authorization graph on create and removes them on delete, separating resource lifecycle from explicit ownership: - **Agents and tasks** register in the authorization graph on create and record an explicit owner; on delete they deregister and the owner record is revoked. - **Schedules and agent API keys** register under their parent agent on create and deregister on delete. Permissions cascade from the parent agent, so no separate owner record is written. Registration runs before the resource is persisted, so an authorization failure fails closed with no orphaned row. If the persist (or the Temporal create, for schedules) later fails, a compensating deregister runs. ## Why Giving each verb a single backend keeps the existing ownership writes (`grant`/`revoke`) unchanged and confines the new behavior to the new lifecycle interface (`register`/`deregister`). Explicit ownership is recorded only for agents and tasks; schedules and agent API keys inherit access from their parent agent, so they need no owner record of their own. Issuing the two write kinds independently keeps both backends current, so reads can cut over per account with a clean rollback. ## Testing - Unit and integration tests covering the agent, task, schedule, and api_key authorization writes (register/deregister ordering, fail-closed-before-persist, and compensating cleanup). - `ruff check` / `ruff format`.
104ecbd to
4527982
Compare
rpatel-scale
approved these changes
Jun 9, 2026
4527982 to
99106f8
Compare
Direct agent-by-id and agent-by-name routes now return 404 instead of 403 when the caller isn't authorized for the agent, matching the existing behavior for tasks and api keys. A 403 on a specific id or name reveals that the agent exists, so collapsing denials to 404 stops callers from probing for agents in other tenants. The name routes resolve the name to an id first, so a genuinely missing name still surfaces the normal repository 404. Ticket: AGX1-242
99106f8 to
f0c7c1b
Compare
harvhan
added a commit
that referenced
this pull request
Jun 9, 2026
#271 made denied agent-route access collapse to 404 instead of 403 and updated the unit authz tests, but this events integration test still asserted 403 -- leaving it red on main and on any open PR. Align the expectation and name with the 404 convention. CI confirms the events route already returns 404 for a denied agent (the assertion failed with `404 == 403`); this only updates the test to match the intended behavior.
harvhan
added a commit
that referenced
this pull request
Jun 9, 2026
Resource ownership was not recorded on agentex resource creation under fine-grained access control, due to two independent bugs. Changes: - is_whitelisted_route: use a boundary-aware match (the route itself or a true sub-path) so /agents/register-build authenticates and carries the caller principal, instead of being swept in by the /agents/register prefix. - Resolve the creator from the dict principal context in the agent, agent_api_key, and schedule register helpers; getattr on the untyped dict always returned None and silently skipped registration. - Update the events authz test to expect 404 for denied agent-route access, per the collapse-to-404 convention (#271).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related work
Parent ticket: AGX1-242 - AgentEx authorization dual-write.
This change is part of a 4-PR stack across 3 repos.
Merge/deploy order: scaleapi/scaleapi#146335 is merged; deploy scaleapi/agentex#364, then merge/deploy scaleapi/scale-agentex#270, then this PR. Rollout per account is: enable
fgac-agentex-resources-dual-write, backfill existing resources into Spark, then enablefgac-agentex-resourcesreads.What
Collapses denied direct agent access from 403 to 404 for agent lookups by id, name, and query. Allowed checks pass through unchanged, and list behavior is unchanged (the list route already filters to authorized ids).
Why
Returning 403 for a specific agent id or name reveals that the agent exists; returning 404 makes "not found" and "exists but not visible to this caller" indistinguishable. This is read-side behavior only.
Testing
Greptile Summary
This PR extends the 403→404 collapse to agent resources across all three authorization shortcut helpers (
DAuthorizedId,DAuthorizedQuery,DAuthorizedName), preventing callers from probing cross-tenant agent existence by comparing 403 vs 404 responses. A new_check_agent_or_collapse_to_404helper is introduced, mirroring the existing patterns for tasks and API keys.authorization_shortcuts.py: adds_check_agent_or_collapse_to_404and wires it intoDAuthorizedId,DAuthorizedQuery, andDAuthorizedNameforAgentexResourceType.agent.test_agents_authz.py(new): comprehensive unit tests for all three dependency paths, covering allowed, denied, and absent-name scenarios.test_schedules_authz.py/test_tasks_authz.py: updated expectations to reflect that agent denials now surface asItemDoesNotExistinstead ofAuthorizationError.Confidence Score: 5/5
Safe to merge — the change is narrowly scoped to collapsing agent authorization denials to 404, all three shortcut helpers are updated consistently, and the only pre-existing omission (DAuthorizedBodyId) is unused with agent resources.
Every path that resolves an agent resource by id, query param, or name now routes through _check_agent_or_collapse_to_404. DAuthorizedBodyId is the only helper not updated, but a search of all call sites confirms it is exclusively used with task resources, so no gap exists. Tests are thorough across all three dependency helpers and cover allowed, denied, and absent-name cases.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Incoming request with agent resource] --> B{Which shortcut?} B --> C[DAuthorizedId\nPath param] B --> D[DAuthorizedQuery\nQuery param] B --> E[DAuthorizedName\nName resolve ID] C --> F{resource_type?} D --> F E --> G[repo.get name to resource.id] --> F F -->|agent| H[_check_agent_or_collapse_to_404] F -->|task| I[check_task_or_collapse_to_404] F -->|api_key| J[_check_api_key_or_collapse_to_404] F -->|other| K[authorization.check 403 on denial] H --> L{AuthorizationError?} L -->|yes| M[raise ItemDoesNotExist 404] L -->|no| N[return resource_id]Reviews (11): Last reviewed commit: "feat(agents): collapse denied agent rout..." | Re-trigger Greptile